Config cache for system.xml loader: Speed improvement #1916

lamskoy · 2021-12-21T22:26:21Z

Description (*)

System.xml loading is not cached anywhere. So each time you open backend page in Magento it scans ALL system.xml inside all modules, merges them and doesn't cache this big XML structure.

Header menu is rendered from system.xml contents too, not only config fields and sections!

My PR implements caching of this stuff.

It decreases any admin page load time in about 1.5 sec on second load on my local machine :)

Manual testing scenarios (*)

Open any magento config page twice and measure time of page load

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All automated tests passed successfully (all builds are green)
Add yourself to contributors list

luigifab · 2021-12-22T10:13:51Z

For me:

before / after
From 550/650 ms to 350/450 ms at localhost with PHP 7.4 when only CONFIG cache is enabled.
From ~1700 ms to 1350/1450 ms at localhost with PHP 7.4 when all cache are disabled.

luigifab · 2021-12-22T15:02:47Z

I think that this error is due to this PR (exception.log):

Exception: "$string" parameter for simplexml_load_string is not a string in
  lib/Varien/Simplexml/Config.php:517

Flyingmana · 2021-12-22T20:29:28Z

From ~1700 ms to 1350/1450 ms at localhost with PHP 7.4 when all cache are disabled.

sounds like there is still a check missing, to not cache it when the cache is disabled?

elidrissidev · 2021-12-23T15:46:21Z

Agree with @Flyingmana, I don't see any code that checks if the cache is enabled. Example from Mage_Api_Model_Config:

magento-lts/app/code/core/Mage/Api/Model/Config.php

Lines 58 to 70 in 4e7a8e1

    
           if (Mage::app()->useCache('config_api')) { 
        
               if ($this->loadCache()) { 
        
                   return $this; 
        
               } 
        
           } 
        
           $config = Mage::getConfig()->loadModulesConfiguration('api.xml'); 
        
           $this->setXml($config->getNode('api')); 
        
           if (Mage::app()->useCache('config_api')) { 
        
               $this->saveCache(); 
        
           } 
        
           return $this;

lamskoy · 2021-12-31T11:01:05Z

Agree with @Flyingmana, I don't see any code that checks if the cache is enabled. Example from Mage_Api_Model_Config:

magento-lts/app/code/core/Mage/Api/Model/Config.php

Lines 58 to 70 in 4e7a8e1

if (Mage::app()->useCache('config_api')) {

if ($this->loadCache()) {

return $this;

}

}

$config = Mage::getConfig()->loadModulesConfiguration('api.xml');

$this->setXml($config->getNode('api'));

if (Mage::app()->useCache('config_api')) {

$this->saveCache();

}

return $this;

Yeah, you're right. Pushed new commit with fix. Bound to 'config' cache

elidrissidev · 2021-12-31T12:09:50Z

Since you're including it with the CONFIG tag, can you also update the description to include system.xml? Something like:

-System(config.xml, local.xml) and modules configuration files(config.xml).
+System(config.xml, local.xml) and modules configuration files(config.xml, system.xml).

app/code/core/Mage/Adminhtml/Model/Config.php

lamskoy · 2022-01-04T20:49:38Z

Done with description
"System(config.xml, local.xml) and modules configuration files(config.xml, system.xml)"

app/code/core/Mage/Core/etc/config.xml

luigifab · 2022-01-28T11:13:37Z

I still get the following error (I didn't searched why - PHP 8.0): Exception: "$string" parameter for simplexml_load_string is not a string in lib/Varien/Simplexml/Config.php:517
After clearing cache and after refresh any configuration pages.

tmotyl · 2022-01-28T12:19:35Z

I think this chnage deservces an comment in readme/changelog, as it changes an expected behaviour developers have -> now they will need to clear the cache after changing system.xml

app/code/core/Mage/Adminhtml/Model/Config.php

fballiano · 2022-06-07T16:03:59Z

is phpstan right about this?

elidrissidev · 2022-06-07T16:07:16Z

Yes, because that parameter is not passed to the parent class.

code was updated

elidrissidev

Two more changes to please phpstan

app/code/core/Mage/Adminhtml/Model/Config.php

elidrissidev · 2022-06-08T12:01:47Z

@fballiano What do you think about mentioning this change in README under "Between Magento 1.9.4.5 and OpenMage 19.x" as tmotyl suggested?

fballiano · 2022-06-08T12:57:10Z

@elidrissidev added info to README ;-)

elidrissidev

Tested once again and everything works well for me, and also a big improvement in loading speed across the Admin area!

github-actions · 2022-06-09T16:47:41Z

Unit Test Results

1 files ±0 1 suites ±0 0s ⏱️ ±0s
0 tests ±0 0 ✔️ ±0 0 💤 ±0 0 ❌ ±0
7 runs ±0 5 ✔️ ±0 2 💤 ±0 0 ❌ ±0

Results for commit 14cf6fa. ± Comparison against base commit 417a20d.

Config cache for system.xml loader

1fbb61e

github-actions bot added the Component: Adminhtml Relates to Mage_Adminhtml label Dec 21, 2021

Uses cache added, libxml internal errors for loading cache string

9fb0b70

elidrissidev requested changes Dec 31, 2021

View reviewed changes

app/code/core/Mage/Adminhtml/Model/Config.php Show resolved Hide resolved

Cache description for system.xml cache

a6297b0

github-actions bot added the Component: Core Relates to Mage_Core label Jan 4, 2022

luigifab reviewed Jan 5, 2022

View reviewed changes

app/code/core/Mage/Core/etc/config.xml Outdated Show resolved Hide resolved

Whitespaces

1ba7c34

elidrissidev previously requested changes Jan 28, 2022

View reviewed changes

app/code/core/Mage/Adminhtml/Model/Config.php Outdated Show resolved Hide resolved

luigifab added 2 commits May 25, 2022 09:40

Check if $xmlString is not empty

b088605

Add text to CSV

6afcb3e

github-actions bot added the translations Relates to app/locale label May 25, 2022

luigifab previously approved these changes May 25, 2022

View reviewed changes

luigifab added the performance Performance related label May 25, 2022

luigifab mentioned this pull request May 25, 2022

Remove frontend_type tag for text #2148

Merged

4 tasks

Removed unused parameter

ac00113

fballiano dismissed luigifab’s stale review via ac00113 June 8, 2022 10:38

fballiano requested review from luigifab and elidrissidev June 8, 2022 10:39

fballiano previously approved these changes Jun 8, 2022

View reviewed changes

elidrissidev reviewed Jun 8, 2022

View reviewed changes

app/code/core/Mage/Adminhtml/Model/Config.php Outdated Show resolved Hide resolved

app/code/core/Mage/Adminhtml/Model/Config.php Outdated Show resolved Hide resolved

Resolve phpstan doc block issues

0fba5a3

elidrissidev dismissed fballiano’s stale review via 0fba5a3 June 8, 2022 11:53

fballiano previously approved these changes Jun 8, 2022

View reviewed changes

Added info about this PR to README

90dfd4c

fballiano dismissed their stale review via 90dfd4c June 8, 2022 12:53

github-actions bot added the documentation label Jun 8, 2022

Merge branch '1.9.4.x' into systemxml_config_cache

0dace6a

fballiano approved these changes Jun 8, 2022

View reviewed changes

elidrissidev approved these changes Jun 8, 2022

View reviewed changes

luigifab approved these changes Jun 9, 2022

View reviewed changes

fballiano merged commit 14cf6fa into OpenMage:1.9.4.x Jun 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config cache for system.xml loader: Speed improvement #1916

Config cache for system.xml loader: Speed improvement #1916

lamskoy commented Dec 21, 2021 •

edited

Loading

luigifab commented Dec 22, 2021 •

edited

Loading

luigifab commented Dec 22, 2021

Flyingmana commented Dec 22, 2021

elidrissidev commented Dec 23, 2021 •

edited

Loading

lamskoy commented Dec 31, 2021

elidrissidev commented Dec 31, 2021

lamskoy commented Jan 4, 2022 •

edited

Loading

luigifab commented Jan 28, 2022 •

edited

Loading

tmotyl commented Jan 28, 2022

fballiano commented Jun 7, 2022

elidrissidev commented Jun 7, 2022

elidrissidev left a comment

elidrissidev commented Jun 8, 2022

fballiano commented Jun 8, 2022

elidrissidev left a comment

github-actions bot commented Jun 9, 2022

Config cache for system.xml loader: Speed improvement #1916

Config cache for system.xml loader: Speed improvement #1916

Conversation

lamskoy commented Dec 21, 2021 • edited Loading

Description (*)

Manual testing scenarios (*)

Contribution checklist (*)

luigifab commented Dec 22, 2021 • edited Loading

luigifab commented Dec 22, 2021

Flyingmana commented Dec 22, 2021

elidrissidev commented Dec 23, 2021 • edited Loading

lamskoy commented Dec 31, 2021

elidrissidev commented Dec 31, 2021

lamskoy commented Jan 4, 2022 • edited Loading

luigifab commented Jan 28, 2022 • edited Loading

tmotyl commented Jan 28, 2022

fballiano commented Jun 7, 2022

elidrissidev commented Jun 7, 2022

elidrissidev left a comment

Choose a reason for hiding this comment

elidrissidev commented Jun 8, 2022

fballiano commented Jun 8, 2022

elidrissidev left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 9, 2022

Unit Test Results

lamskoy commented Dec 21, 2021 •

edited

Loading

luigifab commented Dec 22, 2021 •

edited

Loading

elidrissidev commented Dec 23, 2021 •

edited

Loading

lamskoy commented Jan 4, 2022 •

edited

Loading

luigifab commented Jan 28, 2022 •

edited

Loading